Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load Content Blocks in parallel to data fetching on Field view #1077

Merged
merged 1 commit into from
May 2, 2019

Conversation

jesstelford
Copy link
Contributor

@jesstelford jesstelford commented May 2, 2019

NOTE: PR best viewed with whitespace changes disabled

Network requests before this PR

Screen Shot 2019-05-02 at 10 32 55 am

Note the api call is blocking the loading of the views.

Network requests after this PR

Screen Shot 2019-05-02 at 10 33 53 am

The api call and the views calls are done in parallel

(NOTE: The api call is starting a bit late due to Chrome's concurrent request limit (6). It will start even earlier on a prod deployment under either HTTP/2, or if the Admin UI is served from a different domain to the API)

More to come

You'll notice this PR only addresses the Content field's blocks and nothing else. I purposely wanted to keep this PR small to guage reception. If it all looks 👍, then I can use the same technique with all the other field types, and for Cell / Filters too.

Motivations

There are two main motivations for this change:

  1. Performance as noted above
  2. The Content Editor needs to be able to deserialise the Slate.js JSON blob returned by the API request. Until it's deserialised, it can't be rendered. But the way the code is structured before this PR; we don't load the views until we have the complete (deserialised) data. 🐓&🥚. This change makes it possible to ensure the deserialisation logic (which the views provide) is preloaded before trying to move on with deserialsation & display.

Inspiration

This solution was partly inspired by Facebook's recent F8 video on how they make their dynamic view loading as performant as possible: https://developers.facebook.com/videos/2019/building-the-new-facebookcom-with-react-graphql-and-relay/

}}
</Render>
</Suspense>
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a future PR, I will move this component somewhere re-usable as the logic is identical for Cell/Filter.

Copy link
Member

@emmatown emmatown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants